Allocation Profiler: Types for all allocations #50337

NHDaly · 2023-06-28T20:32:32Z

A variant on #50333.

Pass the types to the allocator functions.

Before this PR, we were missing the types for allocations in two cases:

allocations from codegen
allocations in gc_managed_realloc_

The second one is easy: those are always used for buffers, right?

For the first one: we extend the allocation functions called from codegen, to take the type as a parameter, and set the tag there.

I kept the old interfaces around, since I think that they cannot be removed due to supporting legacy code?

An example of the generated code:

  %ptls_field6 = getelementptr inbounds {}**, {}*** %4, i64 2
  %13 = bitcast {}*** %ptls_field6 to i8**
  %ptls_load78 = load i8*, i8** %13, align 8
  %box = call noalias nonnull dereferenceable(32) {}* @ijl_gc_pool_alloc_typed(i8* %ptls_load78, i32 1184, i32 32, i64 4366152144) #7

Fixes #43688.
Fixes #45268.

NHDaly · 2023-06-28T21:47:40Z

(Similar issue here, btw):

julia> using Profile
Precompiling Profile
  ✗ Profile
  0 dependencies successfully precompiled in 2 seconds
[ Info: Precompiling Profile [9abbd945-dff8-562f-b5e8-e1ebf5ef1b79]
Call parameter type does not match function signature!
  %"+Main.Base.Dict#12848" = load {}*, {}** @SUM.MainDOT.BaseDOT.DictYY.1284, align 8, !dbg Call parameter type does not match function signature!
  !269, !tbaa !233, !alias.scope !234, !noalias !237, !nonnull !0, !dereferenceable !248, !align !245
 i64  %newstruct = call noalias nonnull dereferenceable(80) {} addrspace(10)* @ijl_gc_pool_alloc_typed(i8* %ptls_load1314, i32% 17 = inttoptr i64 %Tuple1328 to {}*
 i64, i32 80, {}* nonnull %"+Main.B  a%sboxe = .callD icnoalias nonnull dereferenceable(32) {} addrspace(10)* t@#ijl_gc_pool_alloc_typed1(2i8848") #21, *! d%b21g,  i!32269 1184
, i32 32, {}* %17) #21
in function julia_Dict_1299
LLVM ERROR: Broken function found, compilation aborted!
in function jfptr_ht_keyindex2_shorthashNOT._1306
LLVM ERROR: Broken function found, compilation aborted!

EDIT: oops, previous comment was mistaken.

NHDaly · 2023-06-28T21:58:13Z

Aha! I fixed it with a small builder.CreatePtrToInt(tag, T_size). I'm not sure why it was needed here when it wasn't needed elsewhere.. 🤔 but it makes sense I think that it should be needed.

NHDaly · 2023-06-28T22:08:21Z

This PR is actually really nice and simple, and assuming I'm not missing any reason why we were setting the tag in LLVM IR, hopefully this is a nice little change that solves the profiler issue! 😊

Thanks in advance for the review! :)

src/gc.c

vchuravy · 2023-06-29T04:14:44Z

@nanosoldier runbenchmarks(!"scalar", vs=":master")

nanosoldier · 2023-06-29T10:15:24Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

NHDaly · 2023-06-29T14:54:57Z

@vchuravy: What do you make of those benchmark results? There's a surprising amount of improvements, which doesn't really make sense to me, and makes me worried that the results are just quite noisy.

There are more regressions than improvements, to be sure. The worst offender is this one:

["tuple", "reduction", ("sum", "(2,)")]        1.71 (5%) ❌

src/gc.c

vchuravy · 2023-06-30T17:27:44Z

@vchuravy: What do you make of those benchmark results? There's a surprising amount of improvements, which doesn't really make sense to me, and makes me worried that the results are just quite noisy.

There are more regressions than improvements, to be sure. The worst offender is this one:
["tuple", "reduction", ("sum", "(2,)")]        1.71 (5%) ❌	

It's pretty much just noise

vtjnash · 2023-06-30T19:18:59Z

@nanosoldier runbenchmarks("find", vs=":master")

nanosoldier · 2023-06-30T19:37:38Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

vchuravy · 2023-06-30T20:39:55Z

You will also need to adjust the docs of the allocation profiler.

NHDaly · 2023-06-30T22:44:56Z

And fix the generated code tests, once we decide what we want the final codegen behavior to be.

doc/src/manual/profile.md

Pass the types to the allocator functions. ------- Before this PR, we were missing the types for allocations in two cases: 1. allocations from codegen 2. allocations in `gc_managed_realloc_` The second one is easy: those are always used for buffers, right? For the first one: we extend the allocation functions called from codegen, to take the type as a parameter, and set the tag there. I kept the old interfaces around, since I think that they cannot be removed due to supporting legacy code? ------ An example of the generated code: ```julia %ptls_field6 = getelementptr inbounds {}**, {}*** %4, i64 2 %13 = bitcast {}*** %ptls_field6 to i8** %ptls_load78 = load i8*, i8** %13, align 8 %box = call noalias nonnull dereferenceable(32) {}* @ijl_gc_pool_alloc_typed(i8* %ptls_load78, i32 1184, i32 32, i64 4366152144) #7 ``` Fixes JuliaLang#43688. Fixes JuliaLang#45268. Co-authored-by: Valentin Churavy <[email protected]>

NHDaly marked this pull request as draft June 28, 2023 20:32

NHDaly mentioned this pull request Jun 28, 2023

Allocation Profiler: Types for all allocations #50333

Closed

NHDaly requested a review from vchuravy June 28, 2023 22:01

NHDaly changed the title ~~Allocation Profiler: Types for all allocations -- Alternative approach~~ Allocation Profiler: Types for all allocations -- Alternative approach: Pass the type when allocating the object. Jun 28, 2023

NHDaly force-pushed the nhd-alloc-profiler-types-2 branch from acdd711 to eb2c692 Compare June 28, 2023 22:06

NHDaly marked this pull request as ready for review June 28, 2023 22:07

NHDaly requested a review from d-netto June 28, 2023 22:08

vchuravy reviewed Jun 29, 2023

View reviewed changes

src/gc.c Outdated Show resolved Hide resolved

NHDaly commented Jun 29, 2023

View reviewed changes

src/gc.c Outdated Show resolved Hide resolved

NHDaly requested review from vchuravy and vtjnash June 29, 2023 16:10

NHDaly force-pushed the nhd-alloc-profiler-types-2 branch from 45f3f5e to bd96a56 Compare July 6, 2023 03:41

vtjnash approved these changes Jul 6, 2023

View reviewed changes

NHDaly mentioned this pull request Jul 7, 2023

Improve documentation for new allocation profiler #45268

Closed

NHDaly force-pushed the nhd-alloc-profiler-types-2 branch 4 times, most recently from 71a099e to a22ba5a Compare July 7, 2023 18:26

vchuravy reviewed Jul 7, 2023

View reviewed changes

doc/src/manual/profile.md Outdated Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allocation Profiler: Types for all allocations #50337

Allocation Profiler: Types for all allocations #50337

NHDaly commented Jun 28, 2023 •

edited

Loading

NHDaly commented Jun 28, 2023 •

edited

Loading

NHDaly commented Jun 28, 2023

NHDaly commented Jun 28, 2023

vchuravy commented Jun 29, 2023

nanosoldier commented Jun 29, 2023

NHDaly commented Jun 29, 2023

vchuravy commented Jun 30, 2023

vtjnash commented Jun 30, 2023

nanosoldier commented Jun 30, 2023

vchuravy commented Jun 30, 2023

NHDaly commented Jun 30, 2023

Allocation Profiler: Types for all allocations #50337

Allocation Profiler: Types for all allocations #50337

Conversation

NHDaly commented Jun 28, 2023 • edited Loading

NHDaly commented Jun 28, 2023 • edited Loading

NHDaly commented Jun 28, 2023

NHDaly commented Jun 28, 2023

vchuravy commented Jun 29, 2023

nanosoldier commented Jun 29, 2023

NHDaly commented Jun 29, 2023

vchuravy commented Jun 30, 2023

vtjnash commented Jun 30, 2023

nanosoldier commented Jun 30, 2023

vchuravy commented Jun 30, 2023

NHDaly commented Jun 30, 2023

NHDaly commented Jun 28, 2023 •

edited

Loading

NHDaly commented Jun 28, 2023 •

edited

Loading